Skip to content

test: remove four file URL tests that stronger tests cover - #904

Merged
Brooooooklyn merged 2 commits into
mainfrom
claude/prune-tests-subset-file-url-tests
Nov 7, 2026
Merged

Brooooooklyn merged 2 commits into
mainfrom
claude/prune-tests-subset-file-url-tests

Conversation

@Brooooooklyn

Copy link
Copy Markdown
Member

Requested by Brooklyn · project thread

Before: four one-line file URL tests from #760 (windows_drive_url_parses, windows_drive_path_generates, windows_unc_url_parses_to_unc_path, non_windows_behavior_is_unchanged) each checked a strict subset of what another test in the same module checks. No regression fails one of them without also failing a stronger test.

After: those four tests are gone. Only #[cfg(test)] modules in src/lib.rs and src/windows_file_url.rs change.

How: each removal was defended by mutating the product code with the most realistic regression it could catch, running cargo test --all-features --lib, and reverting.

Note: the CI job "Test WASI target" (step "Test bindings") already fails on main, since ffa98c1 (#901, 2026-11-01). It will fail here too; this PR does not touch it.

Totals

  • Files touched: 2, test modules only.
  • Cases deleted: 4 of 698 (Rust lib 57, Vitest 599, integrate-module 20, integrate-module-bundler 22). After: 694.
  • Cases folded: 0.
  • Net lines: −25 (+0 / −25).
  • Runtime saved: under 1 ms (each removed case reports <0.000s).

Removals

  1. windows_drive_url_parses (file:///C:/a/b.ts → C:/a/b.ts). Keeping the leading / in the drive form also fails windows_scheme_is_case_insensitive, windows_query_and_fragment_are_not_part_of_the_path, windows_dot_segments_are_normalized and windows_drive_path_is_percent_encoded.
  2. windows_drive_path_generates (C:\a\b.ts → file:///C:/a/b.ts). Not converting backslashes also fails windows_drive_path_is_percent_encoded and windows_verbatim_prefix_is_ignored; converting only the first one also fails windows_drive_path_is_percent_encoded, whose c:\a:b\x.ts row covers the same path.
  3. windows_unc_url_parses_to_unc_path (a three-segment UNC URL). Converting only the first separator, the one thing its depth adds, also fails windows_unc_url_decodes_escapes, windows_mapping_interactions and windows_dot_segments_are_normalized.
  4. non_windows_behavior_is_unchanged (POSIX file_url_to_path for a plain and a %20 path). It was added with the fix for Windows UNC file URLs are not resolved or generated correctly #744, Windows URL rules leaking into POSIX paths. That bug's regression (POSIX file_url_to_path calling windows_file_url::url_to_path) also fails posix_file_url_round_trips, watch_report_urls_are_encoded_and_absolute_only and diagnostics_render_location_and_code_frame. Dropping '0' from hex_digit, skipping decoding, and breaking the no-% fast path each also fail other tests (5, 3 and posix_file_url_round_trips).

Kept after defense

  • windows_escaped_unc_path_round_trips: its decode-back half is the only UNC-form check of %25.

Case counts

suite before after delta
src/windows_file_url.rs tests 32 29 −3
src/lib.rs tests 25 24 −1
integrate-vitest, integrate-module, integrate-module-bundler 641 641 0

0 failures before and after.

What ran and what did not

  • At ffa98c1: all three JS suites against a debug addon, and cargo test --all-features with --report-time.
  • On this branch: cargo test --all-features --lib (53 passed), cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings and git diff --check, all clean.
  • Not run: the 3 loader_context tests under --no-default-features, Windows, the WASI build and the bench. The JS suites were not re-run after the change, which touches only #[cfg(test)] code.

Open pull requests: neither open PR (#903, #894) touches a test file.

Ages: all four tests were added on 2026-09-14 in #760 and are unchanged since. None is in a permission or security suite.

To revert: git revert 53ef6d7.


Generated by Claude Code

`windows_drive_url_parses`, `windows_drive_path_generates`,
`windows_unc_url_parses_to_unc_path` and `non_windows_behavior_is_unchanged`
were the first, one-line file URL tests from #760. Each now checks a strict
subset of what a later test in the same module checks, and every
regression they catch also fails one of those tests:

- `windows_drive_url_parses`: `windows_scheme_is_case_insensitive`,
  `windows_query_and_fragment_are_not_part_of_the_path`,
  `windows_dot_segments_are_normalized`, `windows_drive_path_is_percent_encoded`
- `windows_drive_path_generates`: `windows_drive_path_is_percent_encoded`,
  `windows_verbatim_prefix_is_ignored`
- `windows_unc_url_parses_to_unc_path`: `windows_unc_url_decodes_escapes`,
  `windows_mapping_interactions`, `windows_dot_segments_are_normalized`
- `non_windows_behavior_is_unchanged`: `posix_file_url_round_trips`,
  `watch_report_urls_are_encoded_and_absolute_only`

Only `#[cfg(test)]` modules change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf
@Brooooooklyn Brooooooklyn self-assigned this Nov 4, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-07T08:07:54.198911Z 1165500 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Brooooooklyn
Brooooooklyn merged commit f557fd7 into main Nov 7, 2026
67 checks passed
@Brooooooklyn
Brooooooklyn deleted the claude/prune-tests-subset-file-url-tests branch November 7, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants